-
-
Notifications
You must be signed in to change notification settings - Fork 223
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
BUGFIX: Reduce nodetype schema size #4561
Conversation
With this change the following optimisations are done to improve speed and reduce size of the schema generation: * Abstract nodetypes are not queried anymore for constraints as they are already resolved by the nodetype manager. * Entries in the inheritance map and constraints will be skipped if they don’t contain any data. These optimisations reduce the size of the schema in the Neos.Demo from ~380KB to ~307KB and improve the response time by ~20%. The more nodetypes a project has, the bigger the benefit is.
I wanted to also skip the abstract types in the first loop, but the Neos.UI seems to need at least Edit: I guess the entries in |
Example from a customer project:
Now their biggest chunk is broken nodetypes which are neither abstract nor have super types. Those are massively enriched by the schema builder with useless information. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all makes sense to me, thanks! I kinda see the need to rethink how this schema works, or rather how we build it, seems super convoluted, but that's another task.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks reasonable, I left just one small question inline…
Broke some tests, will check. |
… nodetype anymore
@mhsdesign Tests run again, are you also ok with the change? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes ;)
As we decided to move this either way to the neos ui, i eased the upmerge by ignoring your change for now. See issue neos/neos-ui#3658 |
@Sebobo @mhsdesign It seems this breaks a feature of https://github.com/sitegeist/Sitegeist.Archaeopteryx and I'm not sure how exactly. In this package, I can configure the In Neos 8.3.6, I could use Abstract Node Types in the configuration:
From 8.3.7, this does not work anymore. In 8.3.6, the Node Type Schema returned abstract types including their children:
In 8.3.7, this block is completely missing. Is there another way Sitegeist.Archaeopterix could resolve the node types beloging to such an Abstract type? |
Yes there is a workaround in neos/neos-ui#3677 documented. |
With this change the following optimisations are done to improve speed and reduce size of the schema generation:
Abstract nodetypes are not queried anymore for constraints as they are already resolved by the nodetype manager.
Entries in the inheritance map and constraints will be skipped if they don’t contain any data.
These optimisations reduce the size of the schema in the Neos.Demo from ~357KB to ~300KB and improve the response time by ~20% in my tests.
The more nodetypes a project has, the bigger the benefit is.
Review instructions
Everything should work the same, adding nodes, constraints, etc.
Checklist
FEATURE|TASK|BUGFIX
!!!
and have upgrade-instructions